fix(components): combobox honours options[].disabled - #8142
Conversation
`options[].disabled` was declared by `@object-ui/types` and validated by the
zod mirror, but `Combobox` never read it: each option became a `CommandItem`
carrying `key`, `value` and `onSelect` only, so an option authored
`disabled: true` passed validation, type-checked against the published
`ComboboxSchema`, and rendered as an ordinary selectable option.
The component now sets `disabled={option.disabled}`, the spelling the sibling
select renderer already uses on `SelectItem`. cmdk 1.1.1 needs nothing more:
a disabled `Command.Item` renders with `onClick` undefined, never registers
the `cmdk-item-select` listener Enter dispatches, and is excluded from the
valid-item selector the arrow keys walk — so a refusal inside `onSelect`
would have been unreachable code. Measured, not assumed.
The pin asserts both halves, because an attribute-only pin passes on "styled
disabled but still clickable": the option carries `data-disabled` /
`aria-disabled`, AND neither a click nor Enter fires `onValueChange`. Two
control legs keep those negatives from being vacuous.
Also refreshes the `ComboboxOption` block in the combobox docs page, which
still documented the shape as `{ value; label }`. It sits in a plaintext
fence, so no docs gate reads it.
Refs: objectui#7687
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
`component-docs-disabled-inherited-7239.test.ts` is a census over `content/docs/components`: it classifies every documented `disabled?:` row as INHERITED or INDEPENDENT and pins the population by measurement. Documenting `ComboboxOption.disabled` adds a real 23rd row, so the census failed with the row unclassified and the population one short. The row is claimed as INDEPENDENT on the file's own stated criterion, not by bumping the count: the shipped `ComboboxOption` declares `disabled` itself and does NOT extend `BaseSchema`, so the narrow `boolean` spelling on the page is the correct one and the second INDEPENDENT assertion holds for it unchanged. Counts move consistently — rows 22 to 23, independent 8 to 9, inherited unchanged at 14. A new `## Amendments` section records why the ledger moved, so a later reader can tell a deliberate claim from a number someone bumped to get back to green. It also corrects a belief the #7687 card recorded: the page's `plaintext` fence does keep the doc-TYPE gates from reading it, but this census reads the `.mdx` files as text, so a docs edit under `content/docs/components` that touches a `disabled` row is answerable to `packages/types`. Refs: objectui#7687 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contract review of the patch-round delta — PASS · and the landing decision, with the one red namedReviewed-by: Clause-②: NO, and this is the reason no The delta reviewed:
|
| requirement | what the round did | verdict |
|---|---|---|
| classify, ⛔ don't bump | ComboboxOption placed in the INDEPENDENT table on the file's own criterion — verified declared only in packages/types/src/form.ts, carries no extends BaseSchema, declares disabled itself |
✅ |
| counts move consistently | rows 22 → 23, independent 8 → 9, inherited unchanged at 14 — the inherited count holding still is the check that the row went to the right table | ✅ |
| ⛔ no vacuous pass | ⭐ the round confirmed through the verbose reporter that both new assertions actually executed and passed — ⛔ not that a widened count stopped complaining. That is the distinction that separates a claim from a number edited into agreement, and it is the single most important line in the report | ✅ |
| the ledger move is legible later | a new ## Amendments section records why the population moved, so the next reader can tell a deliberate claim from a green-chasing bump |
✅ |
| ⛔ docs line not reverted | intact | ✅ |
changeset honest about @object-ui/types |
explains why that package gets no bump — no shipped type or value moves there, only a test | ✅ |
| PR body corrected where the round falsified it | ⭐ the body now corrects a belief the card itself carried — that a plaintext fence is invisible to CI. True of the doc-TYPE gates, false overall, and the census is the counter-example that proves it |
✅ |
⭐ One declared deviation, accepted: the full local suite aborted at the 560s foreground cap (exit 124), so the round enumerated the doc-reading population by grep and ran all of it — 24 scripts/__tests__ files + 25 package test files + all of packages/types. ✅ Correct call, and correctly declared: a scope narrowed to the package the code fix lives in is precisely what could not have seen this census failure in the first place. Enumerating the population by measurement rather than by guess is the right substitute for a run that does not fit.
CI: converged. 32 checks — 29 success, 3 skipped, 1 failure
The one failure is Live E2E (informational), and I am naming it rather than waving it through.
⛔ It is not this PR's, and that is measured, not assumed. Root-caused today from run 34056438855 job 101549092958: published @objectstack/*@17.2.0 imports createLocalAccountIssuer from @better-auth/core/db, which does not export it ⇒ AuthPlugin failed to load ⇒ Core service missing: auth ⇒ no sys_* table is ever created (no such table: sys_organization / sys_user / sys_position / sys_permission_set) ⇒ the readiness probe times out at 300s ⇒ exit 1. Full evidence on objectui#8084 (comment 5561862079); filed on the producing side as objectstack#16411.
⇒ the failure is in a published backend dependency's module resolution. This PR's diff is one prop read in a combobox, one docs line, and a census classification. There is no mechanism by which it reaches an ESM named-export mismatch in @better-auth/core. The lane is also advisory by its own workflow's words — "informational lane — does not block merge".
⛔ No re-run spent: this is the established-not-mine case, and the standing-down note is on the record above rather than a retry.
Landing
Pre-landing three: ① the delta review above is PASS, no carrier to clear, head b0a6b3c8b unchanged since the review; ② flipping ready and enabling auto-merge (SQUASH) next; ③ queue entry confirmed by ref with a positives-only control. Fixes #7687, so the card closes on merge and this seat strips pm:dispatched.
Generated by Claude Code
Fixes #7687
What was wrong
options[].disabledon acomboboxnode was declared (@object-ui/typesComboboxOption.disabled), validated (ComboboxOptionSchemain the zod mirror, pinned asbooleanon both faces by the objectui#7087 twin-symmetry ruling) and never read. The component mapped each option to aCommandItemcarryingkey,valueandonSelectonly, so an option authored{ value, label, disabled: true }passedsafeValidateSchema, type-checked against the publishedComboboxSchema, and rendered as an ordinary, fully selectable option.Reproduced before touching anything — the pin in this PR, run against the base tree
5b09821ed, fails exactly there:The fix — one prop, the house spelling
disabled={option.disabled}on theCommandItem, which is what the sibling select renderer already does on itsSelectItem(packages/components/src/renderers/form/select.tsx). The styling needed no work: theCommandItemwrapper's className already carrieddata-[disabled=true]:opacity-50anddata-[disabled=true]:pointer-events-none, so only the read was missing.Remedy chosen: honour the key, not retire it. Retiring
disabledfromComboboxOptionand the zod mirror would narrow a published surface and would require the objectui#7087 twin-symmetry pin to be changed; honouring it restores declared = enforced for the cost of one prop. No declared type or value moves —ComboboxOptionand the zod mirror are exactly as they were; the onlypackages/typesfile in this diff is a census test, described below.Measured: the
onSelectguard would have been dead codeThe card left open whether cmdk needs a refusal inside
onSelectin addition to thedisabledprop. Measured against cmdk 1.1.1 rather than assumed — a disabledCommand.Item:onClickundefined, so a click reaches no handler;cmdk-item-selectlistener that Enter dispatches;aria-disabled="true"is filtered out).So the prop alone suppresses selection on both the pointer and the keyboard path, and an extra guard in
onSelectwould be unreachable. It is not added. The reasoning is recorded at the change site so the next reader does not re-open the question, and the behaviour legs of the pin are what keep the measurement honest.The pin asserts behaviour, not just the attribute
⛔ An attribute-only pin passes on "styled disabled but still clickable", which is the exact failure this card is about.
packages/components/src/__tests__/combobox-option-disabled.test.tsxhas five legs:data-disabledandaria-disabledonValueChangeonValueChangedisabledomitted and withdisabled: falseare bothdata-disabled="false", and clicking one DOES fireonValueChangeThe two controls exist because both negatives are vacuous without them — a popover that never opened would satisfy "not called" just as well.
Reverse verification, run from the committed state (mutation proven on disk by a marker count
1 → 0and a blob-hash mismatch against theHEADblob, restored withgit checkout HEAD -- ...and proven clean by an emptygit diff HEAD): removing the one-line read turns all three assertion legs red and leaves both control legs green. That is the shape it has to have — the controls measure the surrounding machinery, not the fix.The docs row, and the census that claims it
content/docs/components/form/combobox.mdxdocumentedComboboxOptionas value-plus-label, missingdisabled. Only that interface block is touched; the rest of the page is untouched, andcontent/docs/releases/is not touched at all.plaintextfence means no CI gate could see it. That is true of the doc-TYPE gates —check-doc-component-typesreads onlytypestring literals, andcheck-doc-snippet-typescompilests/tsxfences, notplaintext— and false overall:packages/types/src/__tests__/component-docs-disabled-inherited-7239.test.tsis a census that reads those.mdxfiles as text, counts every documenteddisabled?:row, and pins the population. It caught the new row, red, onTest (shard 4/4).So the second commit claims the row deliberately. ⛔ Not a count bump: the row is classified on the criterion that file states — the shipped
ComboboxOptiondeclaresdisableditself and does not extendBaseSchema, so it is INDEPENDENT and the narrowbooleanspelling on the page is the correct one. Counts move consistently: rows 22 to 23, independent 8 to 9, inherited unchanged at 14. A new## Amendmentssection in that file records why the ledger moved, so a later reader can tell a deliberate claim from a number bumped to get back to green.@object-ui/typesis deliberately not given its own changeset bump: no shipped type or value moves there, only that test.Why
minorMaking a previously ignored key take effect is a user-visible behaviour change. An author who already writes
disabled: truetoday gets a different combobox after this lands. The changeset body says so and records why the retire remedy was not taken. (majoris forbidden in this repo; a deliberately breaking change shipsminor.)Gates
Re-run at
b0a6b3c8b, after the census fix.vitest run packages/types/src/__tests__/component-docs-disabled-inherited-7239.test.tsComboboxOptionassertions ran, so the row is claimed for the right reasonvitest run packages/types/vitest run packages/components/6ff473171; the second commit touches no file this package reads)scripts/__tests__that readscontent/docs(24 files)content/docs(25 files)packages/typesandpackages/componentstype-checkpnpm lint(full,turbo run lint)check:control-bytescheck:published-dist·check:published-tsconfig-exclude·check:handler-key-readscheck:doc-fences·check:doc-snippets·check:doc-types·check:docs-route-closure·check-doc-linkscheck:changeset-presence·check-changeset-no-major·check-changeset-fixed·check-changeset-overwriteThe doc-reading population above was enumerated by measurement (grep for the tests that read
content/docs) rather than guessed, because a scope narrowed to the package the code fix lives in is exactly what could not see the census failure. A full localpnpm testdoes not fit the container's foreground limit — it is a 4-way shard job on CI — so that enumeration is the declared substitute.Known reds that are not from this branch:
Live E2E (informational)is advisory and red onmainitself (carded objectui#8084), and aBuild and E2Edeath at roughly 11s atpnpm --versionis a corepack download flake.Scope
⛔ No change to the declared surface:
ComboboxOption,ComboboxOptionSchemaand the objectui#7087 twin-symmetry pin are untouched (narrowing them is the retire remedy's territory). ⛔ Not folded into objectui#6349 batch 3 / PR #7691, which deliberately changes no behaviour. ⛔ objectui#7697, the root-barrel export gap, is adjacent and surface-only and is not addressed here — it stays open.Out-of-scope finding filed unassigned while working this card: objectui#8140 — the combobox renderer never reads
ComboboxSchema.defaultValue,.nameor.description, while the sibling select and input renderers read theirs. Same defect class, different member set, deduped against a search whose control fired. ⛔ Not fixed here.⛔ Left as a draft deliberately: CI convergence and landing belong to the dispatching PM.